ENH: New submodule for vtk_to_usd with correct colormap handling#23
ENH: New submodule for vtk_to_usd with correct colormap handling#23
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a comprehensive new vtk_to_usd submodule for converting VTK files to USD format with improved colormap handling and data array preservation. The implementation is based on NVIDIA's ParaViewConnector architecture, adapted for file-based conversion without ParaView dependencies.
Changes:
- Created new modular
vtk_to_usdlibrary with 7 core modules (data_structures, vtk_reader, usd_utils, material_manager, usd_mesh_converter, converter, init) - Removed old monolithic converters (ConvertVTKToUSDBase, ConvertVTKToUSDPolyMesh, ConvertVTKToUSDTetMesh)
- Refactored
ConvertVTKToUSDto use new library internally while maintaining high-level API - Added comprehensive test suite (430 lines)
- Updated documentation and README files
- Updated imports across experiments and workflows
Reviewed changes
Copilot reviewed 40 out of 41 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/physiomotion4d/vtk_to_usd/*.py |
New modular library implementation with data structures, readers, USD utils, materials, and converters |
src/physiomotion4d/convert_vtk_to_usd.py |
Refactored to use vtk_to_usd library as backend |
tests/test_vtk_to_usd_library.py |
Comprehensive test suite for new library |
tests/test_convert_vtk_to_usd.py |
Updated tests for refactored converter |
src/physiomotion4d/__init__.py |
Updated exports, removed old base classes |
data/README.md |
Enhanced documentation with download status |
| Various notebooks | Updated import statements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| except Exception: | ||
| pass |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | |
| pass | |
| except Exception as exc: | |
| self.log_warning( | |
| f"Failed to clear existing 'displayColor' time samples: {exc}" | |
| ) |
| elif self.settings.compute_normals: | ||
| logger.debug("Computing normals for mesh") | ||
| # Normals will be computed by renderer or in post-process | ||
| pass |
There was a problem hiding this comment.
Unnecessary 'pass' statement.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 47 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Add time samples for subsequent steps | ||
| for mesh_data, time_code in zip( | ||
| mesh_data_sequence[1:], time_codes[1:], strict=False |
There was a problem hiding this comment.
The strict=False parameter in zip() is only available in Python 3.10+. This will cause a TypeError in Python 3.9 and earlier. Consider either removing the strict parameter or checking that the lengths match before the loop, or update the minimum Python version requirement if Python 3.10+ is required.
| def __post_init__(self) -> None: | ||
| """Validate data shape.""" | ||
| if self.data.ndim == 1 and self.num_components == 1: | ||
| # Scalar array | ||
| pass | ||
| elif self.data.ndim == 2 and self.data.shape[1] == self.num_components: | ||
| # Vector/multi-component array | ||
| pass | ||
| else: | ||
| raise ValueError( | ||
| f"Data shape {self.data.shape} incompatible with " | ||
| f"num_components={self.num_components}" | ||
| ) |
There was a problem hiding this comment.
The validation in __post_init__ for GenericArray doesn't handle the case where num_components > 1 but data.ndim == 1. According to the code in usd_mesh_converter.py (lines 217-230), flat 1D arrays can be reshaped into 2D arrays with multiple components. The validation should allow 1D arrays when the length is divisible by num_components, or the reshaping logic in usd_mesh_converter.py should be moved here for consistency.
| dc_attr = display_color_pv.GetAttr() | ||
| for t in list(dc_attr.GetTimeSamples()): | ||
| dc_attr.ClearAtTime(t) | ||
| except Exception: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
Extensive changes to create a vtk_to_usd submodule. Leveraged examples from Omniverse and from Omniverse Connectors (e.g., the ParaView Connector) to adapt to the nuances of VTK and USD (e.g., no data array of higher than 4 dimensions, the period "." is not allowed in a primvar name but is often used in VTK data array names, etc). Also, improved handling of colormaps for the primvars.